Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initialize distribution customizations even before config is applied #9547

Merged
merged 2 commits into from
Dec 3, 2024

Conversation

tjquinno
Copy link
Member

@tjquinno tjquinno commented Nov 30, 2024

Description

Resolves #9546

We added a new data structure, DistributionCustomizations, in Helidon 4.1.4 to support an enhancement in MP metrics 5.1 to allow configuration of percentiles, buckets, etc.

Under normal conditions, the Helidon MP metrics CDI extension invokes this type's init method with an MP config object to prepare the various customizations and all is well.

A user wrote a test that used MP metrics (a timer specifically) but did not mark the test with @HelidonTest so the initialization described above did not occur. That caused an NPE.

We have also since had a report of this happening in an app, in which a CDI extension attempts to register a timer before the metrics CDI extension has prepared metrics properly. As with the test use case above, this means metrics tries to use the instance field before it has been set.

This PR modifies the DistributionCustomizations class to initialize the static (non-final) instance variable at load-time without using config. This resolves the problem the user encountered with the test.

Subsequently the metrics CDI extension overwrites that initial value with one derived from configuration.

Documentation

No impact.

@tjquinno tjquinno self-assigned this Nov 30, 2024
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Nov 30, 2024
@@ -62,7 +62,7 @@ class DistributionCustomizations {
private static final Duration DEFAULT_TIMER_MIN = Duration.ofMillis(5);
private static final Duration DEFAULT_TIMER_MAX = Duration.ofSeconds(10);

private static DistributionCustomizations instance;
private static DistributionCustomizations instance = new DistributionCustomizations();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Could make final, I think.)

@tjquinno tjquinno merged commit 340271b into helidon-io:main Dec 3, 2024
46 checks passed
@tjquinno tjquinno deleted the 4.x-mp-metrics-distr-init branch December 3, 2024 21:36
@tjquinno tjquinno mentioned this pull request Dec 4, 2024
17 tasks
barchetta pushed a commit to barchetta/helidon that referenced this pull request Dec 4, 2024
…elidon-io#9547)

* Initialize distribution customizations even before config is applied

* Improve comment
barchetta added a commit that referenced this pull request Dec 4, 2024
…9547) (#9564)

* Initialize distribution customizations even before config is applied
Co-authored-by: Tim Quinn <[email protected]>
arjav-desai pushed a commit to arjav-desai/helidon that referenced this pull request Dec 11, 2024
…elidon-io#9547)

* Initialize distribution customizations even before config is applied

* Improve comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4.x Provide initial temp config for MP metrics DistributionCustomizations
2 participants